feat: Add check to make sure the target paths are within the allowed mount directory #18
Conversation
within the allowed mount directory Github issue #2
| @@ -171,6 +173,22 @@ QString SSNFSWorker::getPerms(QString path, qint32 uid) { | |||
| } | |||
| } | |||
|
|
|||
| QByteArray SSNFSWorker::isValidPath(QByteArray path){ | |||
| QString newPath = QTextCodec::codecForMib(1015)->toUnicode(path); | |||
There was a problem hiding this comment.
QTextCodec::codecForMib(1015)->toUnicode is not required. Qt has a builtin operator= to automatically convert QByteArray to QString http://doc.qt.io/qt-5/qstring.html#operator-eq-5
| QStack<QString> normalizer; | ||
| // Assuming the path starts and ends with / | ||
| for (int i = 1; i < directories.size()-1; ++i){ | ||
| if (QString::compare(directories.at(i),"..")){ |
There was a problem hiding this comment.
QString::compare is not required Qt has a built in operator== to compare: http://doc.qt.io/qt-5/qstring.html#operator-eq-eq-1.
| @@ -171,6 +173,22 @@ QString SSNFSWorker::getPerms(QString path, qint32 uid) { | |||
| } | |||
| } | |||
|
|
|||
| QByteArray SSNFSWorker::isValidPath(QByteArray path){ | |||
There was a problem hiding this comment.
Why return a QByteArray? This function just performs a boolean (true/false only) check.
|
|
||
| int res; | ||
|
|
||
| struct stat stbuf; | ||
|
|
||
| if (targetPath.isNull()){ |
There was a problem hiding this comment.
If the function is changed to return a boolean, it can just be called from the if.
| QString newPath = QTextCodec::codecForMib(1015)->toUnicode(path); | ||
| QStringList directories = newPath.split("/"); | ||
| QStack<QString> normalizer; | ||
| // Assuming the path starts and ends with / |
There was a problem hiding this comment.
Don't assume anything. In fact, a path that does not start with a / should be specifically reported as invalid.
Also, file paths definitely don't end with a /.
If I removed the qCritical() and qInfo() message, the arguments are not being parsed correctly. So I retained them. Let me know if any changes need to be done.
Github issue #2